Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make meetings wider, higher and add description #291

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

endrebmedhus
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Nov 19, 2021

✔️ Deploy Preview for vedtatt ready!

🔨 Explore the source changes: 9423795

🔍 Inspect the deploy log: https://app.netlify.com/sites/vedtatt/deploys/619abc6db7631d0007320d0a

😎 Browse the preview: https://deploy-preview-291--vedtatt.netlify.app/

Copy link
Contributor

@schjoth schjoth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think these boxes are way too huge when no description is provided, because the boxes are almost only white space. But it looks great when you have a somewhat detailed description, and you are an admin.

With all this space, I think we at least should display the organization name as well.

Random suggestions:

  • Display organization name next to the title in a less aggressive font, that wraps to the next line on mobile?
  • Have the active/admin tags together at the right side next to title? (and have reverse-wrap so that the tags are displayed before the title on mobile)

@@ -94,17 +94,20 @@ const Meeting: React.FC<
<Heading as="h2" fontSize="1.5em">
{title}
</Heading>
<Text fontSize="1em">{formatMeetingTime(new Date(startTime))}</Text>
<Box minH="2rem">{description && <Text>{description}</Text>}</Box>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will show an empty div with 2rem even if description is missing. Is this intended? or should it be:

{description && <Box minH="2rem"><Text>{description}</Text></Box>}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants